Skip to content

feat!: migrate all examples to method dispatch#12

Merged
adnaan merged 1 commit intomainfrom
remove-store-interface
Dec 4, 2025
Merged

feat!: migrate all examples to method dispatch#12
adnaan merged 1 commit intomainfrom
remove-store-interface

Conversation

@adnaan
Copy link
Copy Markdown
Contributor

@adnaan adnaan commented Dec 3, 2025

Summary

Migrate all examples from the Store interface with Change() method to automatic method dispatch.

BREAKING CHANGE: All examples now use method dispatch where actions route to methods by name.

Migrated Examples

Example Before After
login Change() switch Login(), Logout(), ServerWelcome()
trace-correlation Change() switch Increment(), Decrement(), Reset()
production/single-host Change() switch Increment(), Decrement(), Reset()
observability Change() switch Increment(), Decrement(), Reset()
avatar-upload Change() switch UpdateProfile(), UploadAvatarComplete()
testing/01_basic Empty Change() Removed (static page)
graceful-shutdown Change() switch Increment(), Decrement(), Reset()
chat Change() switch Send(), Join(), Leave()

Dependencies

Requires: livetemplate/livetemplate#67

Test plan

  • All examples compile successfully
  • Verified against livetemplate core changes

🤖 Generated with Claude Code

BREAKING CHANGE: All examples now use automatic method dispatch instead
of the Store interface with Change() method.

Migrated examples:
- login: Change() → Login(), Logout(), ServerWelcome()
- trace-correlation: Change() → Increment(), Decrement(), Reset()
- production/single-host: Change() → Increment(), Decrement(), Reset()
- observability: Change() → Increment(), Decrement(), Reset()
- avatar-upload: Change() → UpdateProfile(), UploadAvatarComplete()
- testing/01_basic: Removed empty Change() (static page)
- graceful-shutdown: Change() → Increment(), Decrement(), Reset()
- chat: Change() → Send(), Join(), Leave()

Note: avatar-upload action changed from "upload:avatar:complete" to
"upload_avatar_complete" to match new method dispatch format.

Requires: livetemplate/livetemplate#67

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings December 3, 2025 21:58
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates all examples from using a central Change() method with action dispatch to automatic method dispatch where actions route directly to methods by name. This is a breaking change that simplifies the code structure and makes action handlers more discoverable.

Key Changes:

  • Removed Change() methods with switch statements across all examples
  • Created dedicated action handler methods (e.g., Increment(), Login(), Send())
  • Updated action naming convention from colon-separated to underscore-separated (e.g., upload:avatar:completeupload_avatar_complete)

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
trace-correlation/main.go Migrated to method dispatch with Increment(), Decrement(), and Reset() methods
testing/01_basic/main.go Removed empty Change() method (static page with no actions)
production/single-host/main.go Migrated to method dispatch with Increment(), Decrement(), and Reset() methods
observability/main.go Migrated to method dispatch with Increment(), Decrement(), and Reset() methods
login/main.go Migrated to method dispatch with Login(), Logout(), and ServerWelcome() methods
graceful-shutdown/main.go Migrated to method dispatch with Increment(), Decrement(), and Reset() methods
chat/main.go Migrated to method dispatch with Send(), Join(), and Leave() methods
avatar-upload/main.go Migrated to method dispatch with UpdateProfile() and UploadAvatarComplete() methods; refactored shared logic into private processAvatarUpload() helper

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@adnaan adnaan merged commit 13f715f into main Dec 4, 2025
11 of 15 checks passed
@adnaan adnaan deleted the remove-store-interface branch December 4, 2025 07:41
adnaan added a commit that referenced this pull request Apr 13, 2026
Implements Session 2 of the patterns example plan
(livetemplate/livetemplate docs/proposals/patterns.md): Lists & Data
(#8-11) and Search & Filtering (#12-13). Adds 6 new patterns as
self-contained handler+template+state+tests, plus a Session 1 polish
sweep for issues surfaced during manual review.

## New patterns

- **#8 Delete Row** — per-row delete with `lvt-fx:animate="slide"`
  entry animation. Controller holds a process-wide mutex-protected
  in-memory item store so deletions persist across reloads and
  navigation without needing `lvt:"persist"` struct tags. Includes a
  Restore button that repopulates the store.
- **#9 Click To Load** — button-driven pagination over a 25-item
  dataset (3 pages at size 10).
- **#10 Infinite Scroll** — `<div id="scroll-sentinel">`-driven
  pagination over a 100-item dataset, demonstrating the client's
  IntersectionObserver + `loadMorePending` throttle.
- **#11 Value Select** — cascading Make/Model selects via `Change()`
  auto-wiring. Changing Make auto-selects the first Model so the
  cascade is visibly propagated.
- **#12 Active Search** — debounced live search over a 25-contact
  directory, using `Change()` on `<input type="search">`.
- **#13 URL-Preserved Filters** — bookmarkable filter state via URL
  query parameters, read in `Mount()` with graceful fallback for
  unknown values. Validates status ∈ {all, active, completed} and
  sort ∈ {name, date}.

All 6 patterns follow the Session 1 conventions: per-category
state_*.go and handlers_*.go files, `{{define "content"}}` templates
extending layout.tmpl, `data-key` on range items, chromedp E2E tests
with Initial_Load / UI_Standards / Visual_Check subtests plus
action-specific coverage.

## Session 1 polish sweep

- **Edit Row**: dropped hidden-input ID pattern in favor of button
  `value` attribute (`<button name="edit" value="{{.ID}}">` →
  `ctx.GetString("value")`), matching the documented Tier 1 pattern
  in progressive-complexity-reference.md. Same change applied to
  Delete Row's new template.
- **Click To Edit**: wrapped view-mode Edit button in a form for Tier 1
  no-JS compliance (fixes Session 1 tracker issue #66).
- **Bulk Update**: fixed spurious "Updated N user(s)" flash always
  reporting the total user count. Now counts actual checkbox changes
  and shows a "No changes" info flash when none changed. Added
  `{{.lvt.FlashTag "info"}}` to the template since only success was
  previously rendered.

## Cross-handler navigation regression test

Added `TestCrossHandlerNavigation/Index_To_Delete_Row_No_Stale_Dom` to
lock in DOM structure (article/h4/row counts) after index→pattern
navigation, catching treeRenderer state-leak bugs that pre-existing
string-contains assertions miss.

## Infrastructure

- `runStandardSubtests(t, ctx, pico, desc)` helper consolidates 12
  copy-pasted `UI_Standards` + `Visual_Check` subtest pairs across
  Session 1 and Session 2 tests. Delete Row retains its hand-rolled
  `UI_Standards` because it needs an animation-wait precheck.
- Data layer cleanup: `buildItemDataset(n, prefix)` shared builder for
  `listDataset` (25) and `infiniteScrollDataset` (100); `slices.Clone`
  everywhere replacing manual make+copy idioms; `slices.Sorted(maps.Keys)`
  for `getCarMakes`; hoisted contactDirectory to package-level var.

## Dependency

CI runs against `@latest` client from CDN. Once
github.com/livetemplate/client#69 (cross-handler nav + infinite scroll
race + animation fixes) is merged and released, CI here will pick up
the new client and pass. Until then, expect test failures on
`TestCrossHandlerNavigation/Index_To_Delete_Row_No_Stale_Dom` and
possibly `TestInfiniteScroll/Auto_Paginate_To_End` against the stale
client.

Tested locally with `LVT_LOCAL_CLIENT` pointing at the client PR's
build: 15 tests pass, ~55s runtime.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
adnaan added a commit that referenced this pull request Apr 14, 2026
* feat: patterns example Session 2 (patterns #8-13) + polish sweep

Implements Session 2 of the patterns example plan
(livetemplate/livetemplate docs/proposals/patterns.md): Lists & Data
(#8-11) and Search & Filtering (#12-13). Adds 6 new patterns as
self-contained handler+template+state+tests, plus a Session 1 polish
sweep for issues surfaced during manual review.

## New patterns

- **#8 Delete Row** — per-row delete with `lvt-fx:animate="slide"`
  entry animation. Controller holds a process-wide mutex-protected
  in-memory item store so deletions persist across reloads and
  navigation without needing `lvt:"persist"` struct tags. Includes a
  Restore button that repopulates the store.
- **#9 Click To Load** — button-driven pagination over a 25-item
  dataset (3 pages at size 10).
- **#10 Infinite Scroll** — `<div id="scroll-sentinel">`-driven
  pagination over a 100-item dataset, demonstrating the client's
  IntersectionObserver + `loadMorePending` throttle.
- **#11 Value Select** — cascading Make/Model selects via `Change()`
  auto-wiring. Changing Make auto-selects the first Model so the
  cascade is visibly propagated.
- **#12 Active Search** — debounced live search over a 25-contact
  directory, using `Change()` on `<input type="search">`.
- **#13 URL-Preserved Filters** — bookmarkable filter state via URL
  query parameters, read in `Mount()` with graceful fallback for
  unknown values. Validates status ∈ {all, active, completed} and
  sort ∈ {name, date}.

All 6 patterns follow the Session 1 conventions: per-category
state_*.go and handlers_*.go files, `{{define "content"}}` templates
extending layout.tmpl, `data-key` on range items, chromedp E2E tests
with Initial_Load / UI_Standards / Visual_Check subtests plus
action-specific coverage.

## Session 1 polish sweep

- **Edit Row**: dropped hidden-input ID pattern in favor of button
  `value` attribute (`<button name="edit" value="{{.ID}}">` →
  `ctx.GetString("value")`), matching the documented Tier 1 pattern
  in progressive-complexity-reference.md. Same change applied to
  Delete Row's new template.
- **Click To Edit**: wrapped view-mode Edit button in a form for Tier 1
  no-JS compliance (fixes Session 1 tracker issue #66).
- **Bulk Update**: fixed spurious "Updated N user(s)" flash always
  reporting the total user count. Now counts actual checkbox changes
  and shows a "No changes" info flash when none changed. Added
  `{{.lvt.FlashTag "info"}}` to the template since only success was
  previously rendered.

## Cross-handler navigation regression test

Added `TestCrossHandlerNavigation/Index_To_Delete_Row_No_Stale_Dom` to
lock in DOM structure (article/h4/row counts) after index→pattern
navigation, catching treeRenderer state-leak bugs that pre-existing
string-contains assertions miss.

## Infrastructure

- `runStandardSubtests(t, ctx, pico, desc)` helper consolidates 12
  copy-pasted `UI_Standards` + `Visual_Check` subtest pairs across
  Session 1 and Session 2 tests. Delete Row retains its hand-rolled
  `UI_Standards` because it needs an animation-wait precheck.
- Data layer cleanup: `buildItemDataset(n, prefix)` shared builder for
  `listDataset` (25) and `infiniteScrollDataset` (100); `slices.Clone`
  everywhere replacing manual make+copy idioms; `slices.Sorted(maps.Keys)`
  for `getCarMakes`; hoisted contactDirectory to package-level var.

## Dependency

CI runs against `@latest` client from CDN. Once
github.com/livetemplate/client#69 (cross-handler nav + infinite scroll
race + animation fixes) is merged and released, CI here will pick up
the new client and pass. Until then, expect test failures on
`TestCrossHandlerNavigation/Index_To_Delete_Row_No_Stale_Dom` and
possibly `TestInfiniteScroll/Auto_Paginate_To_End` against the stale
client.

Tested locally with `LVT_LOCAL_CLIENT` pointing at the client PR's
build: 15 tests pass, ~55s runtime.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(patterns/test): harden TestActiveSearch/Clear_Query_Restores_All for CI

CI-specific flake: the test passes locally in ~0.4s but hit the 5s
timeout in CI consistently across both runs on this PR. Root cause is
resource pressure — the failing assertion still showed the previous
"Chen" query state with 1 row, indicating the dispatched `input` event
either never reached the Change auto-wirer debounce or the server
response never made it back within the tight timeout.

Changes:
- Bump WaitForCount timeout from 5s to 10s so CI has headroom under
  orphan-process load.
- Add WaitForWebSocketReady(5s) before the dispatch to ensure the WS
  is fully re-stabilized after Filter_To_Single_Result's SendKeys
  debounce cascade finished.
- Focus the input explicitly before dispatching (some browsers ignore
  events on unfocused inputs under stress).
- Defensively dispatch both `input` AND `change` events — the
  auto-wirer listens on `input` for text inputs, but this is cheap
  insurance against any future event-filter change.

Local: still completes in 0.48s. No regression on the happy path.

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
adnaan added a commit that referenced this pull request Apr 28, 2026
Functional fixes from Claude bot + Copilot:

- Pattern numbering collision: my new headers labeled the Sortable
  example as "Pattern #12", but Active Search already had that number.
  Drop the global numbering from the new code rather than cascade-
  renumbering 25+ comments through unrelated patterns.
- Reorder_DragForward had a comment that contradicted the assertion
  ("task-2, task-3, task-1, ..." vs the actual "task-2, task-1,
  task-3, ..."). Comment was wrong; rewritten to match the algorithm.
- Reorder_DragBackward had been stateful — relied on DragForward
  having run first. Now resets to initial order, which also exposed
  a bug in MY expected order (insert-before-target with srcIdx > tgtIdx
  gives [task-1, task-6, task-2, ...], not [task-6, ...]). Fixed.
- SelfDrop_NoOp ignored the chromedp.Run error from the in-page poll
  (`_ = chromedp.Run(...)`), and the async IIFE returned an unawaited
  Promise that chromedp couldn't unmarshal as bool — silently passing.
  Switched to a Go-side time.Sleep + before/after compare. Errors are
  now captured and t.Fatal'd.
- Reorder subtests no longer depend on each other: each now starts
  with a Reset-to-initial via a shared resetToInitial Tasks block.

Template / docs:

- Hgroup subtitle: replace the "diff engine sends only ['o', newKeys]"
  implementation-detail prose with user-facing copy. Drop the false
  "broadcasts to other tabs" claim — the demo persists across reloads
  and is visible to other tabs on next refresh, but doesn't push live
  updates (no Sync/BroadcastAction).
- Add aria-label="Reorderable task list" to the <ul>.
- Drop what-comments per CLAUDE.md (SortableState struct comment,
  initialSortableItems doc).
- Update SortableController doc to remove the stale "broadcast" claim.

Skipped: aria-live announcement region (separate scope, would need a
server-side state field for the announcement text).

TestSortable still passes (5/5 subtests).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
adnaan added a commit that referenced this pull request Apr 28, 2026
Functional fixes from Claude bot:

- Reorder() guard returns now populate state.Items from snapshot.
  Previously the early-exit paths returned `state` as-is, which the
  framework may have populated with a stale Items slice — the bug
  could overwrite the rendered list with stale data on cross-app
  drags or unknown keys. Snapshot-on-every-return is now the
  invariant, matching how Mount() already worked.
- Reset_RestoresInitialOrder was trivially passing because the
  preceding subtest (SelfDrop_NoOp) calls resetToInitial first and
  the self-drop is a no-op, so by the time Reset_RestoresInitialOrder
  ran the list was already in initial order. Now scrambles the list
  first (drag task-3 onto task-1), then asserts Reset reverts it.
- Subtitle copy: drop "by its handle" (the entire <li> is draggable,
  not just the hamburger glyph — the glyph is decorative). Add
  "(mouse only — no keyboard reorder path in this demo)" to set
  expectations honestly.
- Drop the SortableItem.Key what-comment per CLAUDE.md.

Skipped:
- Renumbering existing patterns (Active Search "#12", URL-Preserved
  Filters "#13"): those labels are pre-existing legacy code outside
  this PR's scope. The numbering scheme is now historical-not-ordinal,
  acknowledged by Sortable having no number.
- Bumping time.Sleep to 1s in SelfDrop_NoOp: 500ms is sufficient in
  practice; longer waits add CI time without clear benefit.
- Refactoring the template to drag-by-handle instead of drag-by-row:
  whole-row draggable is the simpler pedagogical demo. Subtitle copy
  is fixed to no longer mislead users about handle-only behavior.

TestSortable still passes (5/5 subtests).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
adnaan added a commit that referenced this pull request Apr 28, 2026
* feat(patterns): sortable list with HTML5 drag-and-drop

Adds the Sortable List pattern (Lists & Data #12), demonstrating the
new lvt-on:dragstart / lvt-on:dragover / lvt-on:drop event support
shipped in @livetemplate/client (livetemplate/client#101).

The template is a plain <ul> of draggable <li>s, each carrying a
data-key. The client auto-stashes the dragged item's data-key into
DataTransfer on dragstart and lifts it back out on drop, so the
controller's Reorder() method receives a {dragSourceKey, dragTargetKey}
pair without any custom JavaScript. lvt-mod:throttle="100" tames
dragover's high frequency. The diff engine emits only ["o", newKeys]
on the wire — no full re-render.

SortableController follows DeleteRowController's process-wide in-memory
shared-state pattern so reorders persist across reloads and across
tabs (multi-tab broadcast story works for free). A Reset Order button
restores the initial ordering.

E2E coverage exercises Reorder forward, backward, the self-drop no-op
guard, and Reset. Drag is simulated by dispatching real DragEvent
objects with a shared DataTransfer via chromedp.Evaluate — this hits
the production client delegation pipeline rather than bypassing it
via liveTemplateClient.send(), but works around the unreliable HTML5
DnD synthesis through CDP Input.dispatchMouseEvent in headless Docker
Chrome.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(patterns): use empty-action drag bindings (marker pattern)

The Sortable pattern was binding lvt-on:dragstart and lvt-on:dragover
to "reorder", which caused one no-op Reorder call per dragstart plus
~10/sec throttled no-op calls per dragover tick. Drop is the only
event that carries meaningful keys.

Switch to the marker pattern (empty action value) introduced in
@livetemplate/client #101 follow-up: the client still runs the
spec-mandated side-effects (preventDefault, dataTransfer.setData) but
skips the WS round-trip. Net result: exactly one Reorder call per
drop gesture instead of dozens of no-ops.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(patterns): tighten Sortable controller + trim test prose

Code-review pass:

- SortableController.Reorder: single-pass key scan with early break
  once both indices are found (academic for 6 items, real if a future
  example uses a longer list).
- SortableController.Reset: hold the lock across the swap+clone instead
  of releasing and re-acquiring via snapshot().
- Comments: drop the "mirrors DeleteRowController" narration and the
  Reorder algorithm prose that just restated the code; keep the
  client-contract description for ctx field names. Trim the
  simulateDrag and SelfDrop_NoOp test rationale to one line each.

No behavior change. TestSortable still passes (3.4s).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: address bot review on PR #84

Functional fixes from Claude bot + Copilot:

- Pattern numbering collision: my new headers labeled the Sortable
  example as "Pattern #12", but Active Search already had that number.
  Drop the global numbering from the new code rather than cascade-
  renumbering 25+ comments through unrelated patterns.
- Reorder_DragForward had a comment that contradicted the assertion
  ("task-2, task-3, task-1, ..." vs the actual "task-2, task-1,
  task-3, ..."). Comment was wrong; rewritten to match the algorithm.
- Reorder_DragBackward had been stateful — relied on DragForward
  having run first. Now resets to initial order, which also exposed
  a bug in MY expected order (insert-before-target with srcIdx > tgtIdx
  gives [task-1, task-6, task-2, ...], not [task-6, ...]). Fixed.
- SelfDrop_NoOp ignored the chromedp.Run error from the in-page poll
  (`_ = chromedp.Run(...)`), and the async IIFE returned an unawaited
  Promise that chromedp couldn't unmarshal as bool — silently passing.
  Switched to a Go-side time.Sleep + before/after compare. Errors are
  now captured and t.Fatal'd.
- Reorder subtests no longer depend on each other: each now starts
  with a Reset-to-initial via a shared resetToInitial Tasks block.

Template / docs:

- Hgroup subtitle: replace the "diff engine sends only ['o', newKeys]"
  implementation-detail prose with user-facing copy. Drop the false
  "broadcasts to other tabs" claim — the demo persists across reloads
  and is visible to other tabs on next refresh, but doesn't push live
  updates (no Sync/BroadcastAction).
- Add aria-label="Reorderable task list" to the <ul>.
- Drop what-comments per CLAUDE.md (SortableState struct comment,
  initialSortableItems doc).
- Update SortableController doc to remove the stale "broadcast" claim.

Skipped: aria-live announcement region (separate scope, would need a
server-side state field for the announcement text).

TestSortable still passes (5/5 subtests).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: address round-2 bot review on PR #84

Functional fixes from Claude bot:

- Reorder() guard returns now populate state.Items from snapshot.
  Previously the early-exit paths returned `state` as-is, which the
  framework may have populated with a stale Items slice — the bug
  could overwrite the rendered list with stale data on cross-app
  drags or unknown keys. Snapshot-on-every-return is now the
  invariant, matching how Mount() already worked.
- Reset_RestoresInitialOrder was trivially passing because the
  preceding subtest (SelfDrop_NoOp) calls resetToInitial first and
  the self-drop is a no-op, so by the time Reset_RestoresInitialOrder
  ran the list was already in initial order. Now scrambles the list
  first (drag task-3 onto task-1), then asserts Reset reverts it.
- Subtitle copy: drop "by its handle" (the entire <li> is draggable,
  not just the hamburger glyph — the glyph is decorative). Add
  "(mouse only — no keyboard reorder path in this demo)" to set
  expectations honestly.
- Drop the SortableItem.Key what-comment per CLAUDE.md.

Skipped:
- Renumbering existing patterns (Active Search "#12", URL-Preserved
  Filters "#13"): those labels are pre-existing legacy code outside
  this PR's scope. The numbering scheme is now historical-not-ordinal,
  acknowledged by Sortable having no number.
- Bumping time.Sleep to 1s in SelfDrop_NoOp: 500ms is sufficient in
  practice; longer waits add CI time without clear benefit.
- Refactoring the template to drag-by-handle instead of drag-by-row:
  whole-row draggable is the simpler pedagogical demo. Subtitle copy
  is fixed to no longer mislead users about handle-only behavior.

TestSortable still passes (5/5 subtests).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: address round-3 bot review on PR #84

- Subtitle: also document that there's no visual drop-zone highlight
  in this v1 demo (the framework doesn't currently expose a hover
  class hook for drag targets, and adding one would require client
  changes outside this example's scope).
- SelfDrop_NoOp time.Sleep: bump to 1s to give loaded CI runners
  headroom for any spurious round-trip to surface in the assertion.
- Reorder() docstring: trim the second half that walked through the
  algorithm; keep the non-obvious invariant about not trusting
  framework-provided state on early-exit paths.

Skipped:
- Adding a Reorders counter field to SortableState as a positive
  assertion replacement for time.Sleep — that pollutes the wire
  format with a test-only field and adds non-trivial state-design
  complexity for marginal robustness improvement.

TestSortable still passes (5/5 subtests).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: address round-4 bot review on PR #84

- Add aria-description on the <ul> so AT users get an explicit notice
  about the missing keyboard reorder path, instead of hitting a silent
  dead-end on draggable items.
- Tighten the Reorder() doc comment to name the source of the ctx keys
  (the livetemplate/client drag handler reading data-key on the
  dragstart and drop targets).

Skipped:
- dragleave/dragend abort guard: moot — the client only fires the
  Reorder action on actual drop events. Aborted drags (release outside
  the list, ESC) don't fire drop on a registered lvt-on:drop element,
  so Reorder isn't called at all.
- Glyph choice (☰ hamburger vs ⋮ vertical ellipsis): both are commonly
  used as drag-handle indicators. Bikeshedding.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix: address round-5 bot review on PR #84

- aria-description (ARIA 1.3, uneven AT support) → aria-describedby
  pointing to a visually-hidden description element. Reliably
  surfaces the keyboard-reorder limitation to screen readers.
- Trim multi-line comment blocks on SortableController and Reorder
  per CLAUDE.md "one short line max" rule. Keep the load-bearing
  WHY (snapshot-on-every-return invariant) on a single line.

Skipped:
- dragend-without-drop edge case: dataTransfer is per-event, no
  persistent client state to dangle.
- Replacing time.Sleep(1s) with an echo-ping pattern: adds complexity
  for marginal robustness; declined twice already.
- "Subtitle leaks 'diff engine' implementation detail": stale read —
  that prose was removed in commit ff7e36c (round 1).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants